-
Notifications
You must be signed in to change notification settings - Fork 248
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[CS2113-T12-4] WellNUS++ #25
base: master
Are you sure you want to change the base?
[CS2113-T12-4] WellNUS++ #25
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Description of design and implementation is generally well done! Good work! Some diagrams may need to be improve for better readability.
docs/DeveloperGuide.md
Outdated
### Reflection Component | ||
|
||
![Reflection Component Class Diagram](diagrams/ReflectionSequenceDiagram.png) | ||
![Reflection Component Class Diagram](diagrams/ReflectionClassDiagram.png) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
docs/DeveloperGuide.md
Outdated
`LikeCommand` class: <br> | ||
|
||
- Command format: `like <index of question(1~5)>` | ||
- Users can add an introspective question that is generated in the previous set into their favorite list. Since there | ||
will only | ||
be 5 questions per set, the indexes are restricted to integer 1~5. | ||
- `addFavQuestion()` method in `QuestionList` class is used to add and store the data. | ||
- Users can only successfully add a question to favorite list if they have gotten a set of questions previously. | ||
- Every time a question is added into the favorite list, the indexes of this particular question will be stored in data | ||
file straightaway. It prevents data loss due to unforeseen computer shutdown. | ||
|
||
`FavoriteCommand` class: <br> | ||
|
||
- Command format: `fav` | ||
- Users can get the questions in their favorite list. | ||
- `getFavQuestions()` method in `QuestionList` class matches the indexes to corresponding questions and return this set | ||
of questions | ||
back to `FavCommand` for output. As such, `QuestionList` is a dependency for `FavoriteCommand` as well. | ||
|
||
`HomeCommand` class: <br> | ||
|
||
- Command format: `home` | ||
- This command allows users to return back to the main WellNUS++ interface. | ||
- Similar to `GetCommand`, `validateCommand()` method will also be called to validate the command. | ||
- It will then call the class-level method `ReflectionManager.setIsExit()` to terminate the while loop | ||
in `Reflectionmanager`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps it will also be great to include how these classes handle errors? Or if there are any cases (or zero cases) of errors they may need to handle?
|
||
![CommandParser implementation](diagrams/CommandParserSequence.png) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
docs/DeveloperGuide.md
Outdated
|
||
### AtomicHabit Component | ||
|
||
![AtomicHabit Component](diagrams/AtomicHabit.png) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#### Overview | ||
The overall execution lifecycle of the WellNus application involves 4 main components, as shown in the diagram below. | ||
|
||
![Application Lifecycle](diagrams/WellnusSequence.png) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice DG overall! The link to github is a nice touch :)
docs/DeveloperGuide.md
Outdated
|
||
![Application Lifecycle](diagrams/WellnusSequence.png) | ||
|
||
The application begins with a call to `WellNus.start()`, which initialises an instance of `MainManager` and calls the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this be done without since it is a description of the sequence diagram?
docs/DeveloperGuide.md
Outdated
![Reflection Component Class Diagram](diagrams/ReflectionSequenceDiagram.png) | ||
![Reflection Component Class Diagram](diagrams/ReflectionClassDiagram.png) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This diagram is too crowded which makes it hard to read. Could you perhaps abstract some of the details out?
docs/DeveloperGuide.md
Outdated
are not the focus of this section since they are outside of `reflection` package.<br> | ||
<br> | ||
|
||
#### Feature Package (`ReflectionManager`, `ReflectionQuestion`, `QuestionList`, `TextUi`, `RandomNumberGenerator` classes) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could consider the use of UML diagrams to represent class to improve clarity
|
||
#### Integration with WellNUS++ | ||
|
||
![Integration](diagrams/CommandParserClass.png) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This class diagram may not follow the standard UML notation
docs/DeveloperGuide.md
Outdated
### Reflection Component | ||
|
||
![Reflection Component Class Diagram](diagrams/ReflectionSequenceDiagram.png) | ||
![Reflection Component Class Diagram](diagrams/ReflectionClassDiagram.png) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The class diagram here could possibly be abstracted out and separated out into different features since it is not mandatory to include everything. Users may find it hard to read as the text size is a little small
docs/DeveloperGuide.md
Outdated
|
||
### AtomicHabit Component | ||
|
||
![AtomicHabit Component](diagrams/AtomicHabit.png) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To simplify the diagram, maybe not every command has to be shown, which would definitely reduce the amount of arrows
docs/DeveloperGuide.md
Outdated
* **Argument**: A word that is a parameter to a `Main Command` and is prefixed by ` --`. `e.g. --id, --name` | ||
* **Payload**: An (optional) arbitrary sequence of characters immediately following a main command or argument. | ||
The payload will terminate when the user clicks `enter` or separates the payload with another argument | ||
with the `--` delimiter. | ||
|
||
## Instructions for manual testing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe this section could be removed from the developer guide? This is because your user guide already covers the commands, creating duplicate content.
docs/DeveloperGuide.md
Outdated
|
||
For example, | ||
` | ||
$ foo bar --arg1 payload1 payload1--1 --arg2 payload2 --arg3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe this could be included in a code block for easier visualization
* Instead of using electronics with fancy GUI, this CLI app gives computing students an opportunity to minimise digital | ||
interaction which will be beneficial for their wellness. | ||
* The app is gamified to make it more attractive for students to use. Levels and micro-goals incentivise our | ||
users to keep using the app’s features, allowing them to focus on their work and achieve wellness. | ||
|
||
## User Stories |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
User stories language format could be changed to fit the "I want to ..." sentence. For e.g. I want to use keyboard instead of mouse vs I want to I can use keyboard instead of mouse
docs/DeveloperGuide.md
Outdated
### Reflection Component | ||
|
||
![Reflection Component Class Diagram](diagrams/ReflectionSequenceDiagram.png) | ||
![Reflection Component Class Diagram](diagrams/ReflectionClassDiagram.png) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
diagram is messy and hard to visualise. might want to consider removing some of the less important parts and only showing the important features of the diagram
docs/DeveloperGuide.md
Outdated
| v2.0 | Reflective student | I can get the previous questions I viewed | I can re-view these questions | | ||
| v2.0 | Easily distracted computing student | I want to start a timer to keep track of time spent on work | I can do timed-practice | | ||
| v2.0 | Easily distracted computing student | I want to check the time | I can keep track of my pace | | ||
| v2.0 | A regular WellNUS++ user | I wish to have my information stored in the app | I can re-view my past data | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can add priority levels / level of importance to this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(sorry i forgot to submit my review)
developer guide is very informative!
docs/DeveloperGuide.md
Outdated
|
||
### Reflection Component | ||
|
||
![Reflection Component Class Diagram](diagrams/ReflectionSequenceDiagram.png) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel that this might be a little repetitive as each command is executed in a similar way. It also ends up being very long and looks complicated. Maybe you can separate the commands.
docs/DeveloperGuide.md
Outdated
### Reflection Component | ||
|
||
![Reflection Component Class Diagram](diagrams/ReflectionSequenceDiagram.png) | ||
![Reflection Component Class Diagram](diagrams/ReflectionClassDiagram.png) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The association lines are all overlapping, and it is difficult to tell them apart, especially at the top where the label and lines overlap. once again i suggest you can separate into perhaps feature and command diagrams.
docs/DeveloperGuide.md
Outdated
The application begins with a call to `WellNus.start()`, which initialises an instance of `MainManager` and calls the | ||
`MainManager.runEventDriver()` method. | ||
|
||
`MainManager.runEventDriver()` will then take control of user input and provide a basic interface that parses commands |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are mentions of keywords and user inputs that are being tackled but i do not see a string being passed through or a text ui class, maybe u can show this in your digram too?
|
||
![CommandParser implementation](diagrams/CommandParserSequence.png) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well made sequence diagram, helps me to understand how a command is parsed.
Update developerguide
Replace 'Implementation Rationale' with 'Design Considerations' to standardise these section headers.
Improve DeveloperGuide Formatting
…activating helpcommand
…rror Remove extra space printed for MainManger when invalid command is given
…Parser Fix developer guide typos command parser
Fix broken link
Fix single-letter variable name
Document add user story dg
More DG test cases
Update DeveloperGuide Table of Contents
Fix User Guide Quickstart Typo
Fix list indices in DeveloperGuide
Update gradle output jar version
Fix broken link
Fix DG list indices
Fix broken TOC Name
WellNUS++ is a Command Line Interface(CLI) app for NUS Computing students to keep track and improve their physical and mental wellness in various aspects. If you can type fast, WellNUS++ can update their wellness progress faster than traditional Graphical User Interface(GUI) apps.